Skip to content
This repository has been archived by the owner on Feb 24, 2021. It is now read-only.

Add more sensors to Eurotronic SpiritZ #810

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

varet80
Copy link
Contributor

@varet80 varet80 commented Oct 24, 2020

Better Hass support, for graphing and data

To work with entities which represent data, added some sensors for
homeassistant discovery

  • Valve Position: Values 0-100
  • Room temperature: Celsius room temperature
  • Set temp as Sensor for normal and Eco

@robertsLando I would like these to be used from both stella and spiritz. But I do not know how to make them merge an array of sensors and the Climate hashmap

in order words something like:
[SPIRIT_ZWAVE_PLUS] + EUROTRONIC_S_ZWAVE_SENSORS

where the latter can be one hashmap with all sensors.

Better Hass support, for graphing and data

To work with entities which represent data, added some sensors for
homeassistant discovery

- Valve Position: Values 0-100
- Room temperature: Celsius room temperature
- Set temp as Sensor for normal and Eco
@robertsLando
Copy link
Member

Are them using the same values? Because each value can only be binded to a unique Hass discovery

@varet80
Copy link
Contributor Author

varet80 commented Oct 24, 2020

Are them using the same values? Because each value can only be binded to a unique Hass discovery

i have a customDiscovery using these sensors, and some values are also shared (same) with the climate discovery.
the system seems to work properly. in my homeassistant.

  "328-1-3": [
    {
      "type": "climate",
      "object_id": "thermostat",
      "values": [
        "64-1-0",
        "49-1-1",
        "67-1-1",
        "67-1-11"
      ],
      "mode_map": {
        "off": "Off",
        "heat": "Heat",
        "cool": "Heat Eco"
      },
      "setpoint_topic": {
        "Heat": "67-1-1",
        "Heat Eco": "67-1-11"
      },
      "default_setpoint": "67-1-1",
      "discovery_payload": {
        "min_temp": 8,
        "max_temp": 28,
        "modes": [
          "off",
          "heat",
          "cool"
        ],
        "mode_state_topic": "64-1-0",
        "mode_command_topic": true,
        "current_temperature_topic": "49-1-1",
        "temp_step": 0.5,
        "current_temperature_template": "{{ value_json.value }}",
        "temperature_state_template": "{{ value_json.value }}",
        "temperature_command_topic": true
      }
    },
    {
      "type": "sensor",
      "object_id": "target_temperature_heat",
      "discovery_payload": {
        "value_template": "{{ value_json.value }}",
        "device_class": "temperature",
        "unit_of_measurement": "°C",
        "state_topic": "67-1-1"
      },
      "values": [
        "67-1-1"
      ]
    },
    {
      "type": "sensor",
      "object_id": "target_temperature_heat_eco",
      "discovery_payload": {
        "value_template": "{{ value_json.value }}",
        "device_class": "temperature",
        "unit_of_measurement": "°C",
        "state_topic": "67-1-11"
      },
      "values": [
        "67-1-11"
      ]
    },
    {
      "type": "sensor",
      "object_id": "room_temperature",
      "discovery_payload": {
        "value_template": "{{ value_json.value }}",
        "device_class": "temperature",
        "unit_of_measurement": "°C",
        "state_topic": "49-1-1"
      },
      "values": [
        "49-1-1"
      ]
    },
    {
      "type": "sensor",
      "object_id": "valve_position",
      "discovery_payload": {
        "value_template": "{{ value_json.value }}",
        "state_topic": "38-1-0"
      },
      "values": [
        "38-1-0"
      ]
    }
  ],```

@robertsLando
Copy link
Member

@billiaz I need to check the code, I'm sure that in some cases it will work without any problems but on z2m side I only store 1 discovery for each value, I think that for example in the case you press "rediscover node" only the last one will rediscovered for example

@robertsLando
Copy link
Member

Yes like I told you if you check /lib/Gateway.js you will see it has a property named discovered that is a map String --> Object where the key is the valueId string and the value contains the discovery payload. This could work but could also create some problems as I said in my previous comment

@aretakisv
Copy link
Contributor

so this is clearly a z2m bit. are these values used to produce uniqueness or also to discover where they belong?

Asking this, if these are just "labels" might worth to name them differently in this case.

@robertsLando
Copy link
Member

@billiaz @aretakisv them are used to keep track of the discovered values of each node, them are used to remove the discovered values for xample when a node is removed or to rediscover them when the user press on rediscover, also if you check on discovered devices you should not be able to see all of them if some have multiple devices discovered with same valueId. Also when a message is received I check for discovered device of that valueId to see If it need to be parsed (for example rgb values)

@aretakisv
Copy link
Contributor

@robertsLando i am the same person Billiaz or this, just replying from another account.

if the value for instanes is 49-1-1-S would that cause any issues, as teh 49-1-1 topi path do not exist? in that case

@robertsLando
Copy link
Member

@billiaz I think that the only fix would be to make that an array of discovered templates instead of single objects

@aretakisv
Copy link
Contributor

you mean , like my example of customDevices.json right? one array of templates!

ofcourse in the end we submit this as an array as you can see.

@robertsLando
Copy link
Member

you mean , like my example of customDevices.json right? one array of templates!

Nope I mean in Gateway.js, discovered actually keeps track of 1 discovered template foreach value ATM. Needs to be converted in array to support this (but this would need some refactor on the code)

@ivdimitro
Copy link

you mean , like my example of customDevices.json right? one array of templates!

Nope I mean in Gateway.js, discovered actually keeps track of 1 discovered template foreach value ATM. Needs to be converted in array to support this (but this would need some refactor on the code)

I think this will be useful change. I am not sure for other devices but for this thermostat in particular it hinders its functionality. For example the topic 64-1-0 is used for setting the heating mode but also for enabling the direct valve control. The current dimmer on/off functionality is not working.
Another benefit is what @aretakisv is doing to expose some of the readings as sensors. This can be done with custom defined sensors in HA but it is better if they are working correctly with autodiscovery.

Are there plans for changing the implementation in the Gateway.js? Unfortunately my JS is not existing at all.

@robertsLando
Copy link
Member

Are there plans for changing the implementation in the Gateway.js? Unfortunately my JS is not existing at all.

Actually nope because I'm focusing on the transition to https://github.com/zwave-js/zwavejs2mqtt

@github-actions
Copy link

This pull-request is stale because it has been open 90 days with no activity. Remove the stale label or comment or this will be closed in 5 days. To ignore this pull-request entirely you can add the no-stale label

@github-actions github-actions bot added the Stale issues+prs marked as stale label Feb 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Stale issues+prs marked as stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants